Add theme variants and stabilize markdown rendering - #1212

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Greptile Summary

This PR adds two built-in dark themes. The main changes are:

• Indigo Blue and Deep Wine Red theme definitions.
• New theme IDs in the shared type and runtime validator.
• Updated theme count and valid-ID test coverage.

Confidence Score: 5/5

This looks safe to merge after a small visual cleanup.

• The shared registry, type guard, and tests were updated together.
• The only noted issue is a dark accent color that can make active UI states hard to see.

src/shared/themes.ts

Important Files Changed

_{Reviews (1): Last reviewed commit: "feat(themes): add Deep Wine Red theme" | Re-trigger Greptile}

@felipeggv Thanks for the contribution! Four new ultra dark variants is a nice addition, and the mechanics here are clean: you updated the ThemeId union, the isValidThemeId runtime allowlist, the THEMES registry, and both test files together, so the three-way sync stays intact. CI is green across all four checks and there are no merge conflicts. Structurally these follow the olive-nights precedent closely (including the rgba(..., 1) form for accentDim), which is exactly right.

One substantive issue before this can go in, and it's an extension of the point Greptile raised on indigo-blue.

accent contrast is below the visibility threshold on all four themes

Greptile flagged the Indigo accent specifically, but the problem generalizes to the whole set. The accent value is very close in luminance to bgMain in each new theme:

WCAG 1.4.11 asks for 3:1 for non-text UI indicators. olive-nights, the closest existing theme in spirit, clears it at 3.32:1. All four new themes fall short.

This matters more than a typical palette nit because of where accent lands. src/renderer/hooks/ui/useThemeStyles.ts:80 maps the theme's accent straight to --accent-color, and the keyboard focus ring falls back to exactly that variable:

/* src/renderer/index.css:329 */
.focus-ring:focus-visible {
	outline: 2px solid var(--focus-ring-color, var(--accent-color, rgba(99, 102, 241, 0.9)));
	outline-offset: 2px;
}

Maestro is a keyboard-first app, so a focus ring at 1.6:1 against a near-black background is effectively invisible. Users tabbing through the UI on these themes would lose track of where focus is.

Worth noting your text colors are all in great shape (textMain and textDim are 11:1 and up against bgMain, and accentForeground on accent is comfortable everywhere). It's specifically accent that needs lifting.

Here are hue-preserving values that clear 3:1, if useful as a starting point. These just scale luminance and keep your color identity intact:

Feel free to pick your own values, these are only illustrative. The ask is just that accent reaches roughly 3:1 against bgMain so focus and active states stay visible. If you'd rather keep the accents deliberately deep for the aesthetic, the other option is to set an explicit brighter --focus-ring-color for these themes, but bumping accent is the simpler path and keeps the rest of the accent-driven UI (highlights, left borders) legible too.

Minor heads-up, no action needed

Both bots reviewed at commit 3ca0f7c ("Deep Wine Red"), before 990f1ad added the mustard and deep purple variants. That's why their summaries only mention two themes and four IDs. Your diff does correctly define all four, so nothing is actually missing, but the last commit went unreviewed by the bots. That's part of why I checked the full set above.

Once the accent contrast is addressed I'm happy to approve. Thanks again!

Thanks! Picked this up in a PR sweep but it conflicts with rc right now. Mind rebasing on the latest rc? I will review and merge once it is mergeable.

Two Markdown issues remain after the current changes:

1. On an uncached Mermaid update, MermaidRenderer.tsx:383 sets loading but leaves the previous svgContent in state. The loading branch at line 504 only renders when there is no SVG, so diagram A stays visible with no updating state while diagram B renders. I reproduced this in Electron with a delayed real Mermaid render. Please clear the old SVG on a cache miss, or show an explicit updating state, and add a delayed rerender regression.

2. The new cache limits bound entry count, not memory. Each CodeFence key retains the complete code block plus its highlighted HTML, and each Mermaid entry retains the complete chart plus SVG for the renderer lifetime. A small number of large transcript blocks can therefore retain substantial memory after unmount. Please use a byte-budgeted LRU or skip entries above a conservative input/output size.

I did not repeat the existing contrast and rebase feedback.

@felipeggv Thanks again for sticking with this one - the caching work here is still the most valuable part of the PR, and nothing in rc has superseded it. Picking it up in another sweep, so here is a consolidated status of everything outstanding, plus some rebase guidance that should save you real time.

1. Needs a rebase on latest rc (blocking)

rc has moved a lot in the last month and all 11 changed files now conflict:

src/shared/themes.ts
src/shared/theme-types.ts
src/renderer/components/CodeFence/CodeFence.tsx
src/renderer/components/MermaidRenderer.tsx
src/renderer/components/InputArea/components/InputTextarea.tsx
src/renderer/components/InputArea/utils/textareaSizing.ts
+ the 5 corresponding test files

Three things worth knowing before you start, because they change what the resolution should look like:

Drop indigo-blue from this PR. It already landed on rc via #1207, and the definition there is byte-for-byte identical to yours (I diffed the block). Take rc's side wholesale for that theme and keep only deep-wine-red, yellow-dark-mustard, and deep-purple. That removes the bulk of the themes.ts / theme-types.ts conflict.

The theme count test works out unchanged. rc currently expects 21. Adding your remaining three lands on 24, which is exactly what this PR already sets, so src/__tests__/renderer/constants/themes.test.ts needs no further edit once indigo-blue is dropped.

The composer changes need a real re-think, not a mechanical merge. rc has since landed #1169, #1294, #1321, #1329, and #1330, all reworking caret visibility, mention-highlight alignment, and composer selection state in InputTextarea.tsx. Your overlay/selection changes may be partly or wholly superseded. Please re-derive that piece against current rc rather than resolving conflict markers, and drop anything already fixed upstream.

2. Cache entries are written on every streamed chunk

This one sharpens the memory point @jSydorowicz21 raised, and I think it is the most actionable item in the PR.

The highlight effect in CodeFence.tsx depends on the raw code prop, not debouncedCode:

}, [resolvedLang, code, theme.mode]);

Detection is correctly debounced ("streaming doesn't re-detect per char"), but caching is not. So while a fence streams in, every intermediate snapshot calls rememberHighlightedHtml(...) and inserts a distinct entry keyed on the full partial code string. A single 300-line block arriving in ~200 chunks churns ~200 entries through an 80-entry cache, each retaining a progressively longer code string as the key plus its rendered HTML as the value.

Two consequences: the cache thrashes itself during exactly the scenario it was added for, and the retained bytes are far larger than the 80-entry bound suggests. Gating writes on the settled/debounced value, or skipping entries above a conservative size, would fix both at once.

Related and cheap to fix: both caches are FIFO, not LRU. There is no reinsert on a cache hit, so keys().next().value always evicts the oldest insertion regardless of how hot it is. A frequently revisited block can be evicted by cold one-offs. A delete + set on hit makes it a true LRU.

3. Stale Mermaid diagram on a cache miss

Confirmed @jSydorowicz21's first point by reading the diff. The old setSvgContent(null) was removed from the miss path, and the loading gate is now isLoading && !svgContent, so on a cache miss the previous diagram stays on screen with no indication that a new one is rendering.

I read this as intentional anti-flicker, and I do not think reverting to a spinner is right - that would undo the PR's purpose. But silently showing diagram A while B renders is misleading. A subtle updating affordance (reduced opacity, a small spinner in the corner) keeps the anti-flicker win without the lie. A delayed-rerender regression test would be good to have alongside it.

4. Accent contrast: withdrawing this as a blocker

I raised this in my first comment. Since then indigo-blue shipped in #1207 with accent: #1f2f63 unchanged, so the project has effectively set precedent against my own objection and it would be unfair to hold this PR to a stricter bar. Consider it optional. The remaining three accents are still deep enough that the focus ring will be hard to see on them, so if you want to lift them toward ~3:1 against bgMain I would welcome it, but it is your call and I will not block on it.

Summary

Items 1 through 3 are what I would like addressed. Item 1 is the hard blocker since I cannot merge a conflicting branch. If the rebase turns out to be more trouble than it is worth, splitting the CodeFence and Mermaid caching into its own PR against fresh rc would be very welcome - that part is genuinely useful, has no competing implementation upstream, and would avoid the composer churn entirely. Happy to review it quickly either way.